Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backports for 1.8-rc1/beta4 #44789

Merged
merged 68 commits into from
May 26, 2022
Merged

Backports for 1.8-rc1/beta4 #44789

merged 68 commits into from
May 26, 2022

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Mar 29, 2022

Backported PRs:

Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:

DilumAluthge and others added 5 commits March 29, 2022 16:02
Discovered while running the syntax tests with lines 25-28 of
`jlfrontend.scm` commented out. Turned out we didn't handle `.op`
correctly in neither `check-dotop` nor in `deparse`. This meant we just
got `error: malformed expression` instead of an actually useful error
message.

(cherry picked from commit 9112135)
@KristofferC KristofferC added the release Release management and versioning. label Mar 29, 2022
aviatesk and others added 24 commits March 29, 2022 16:15
#44786)

The current naming of `overlayed` is a bit confusing since
`overlayed === true`, which is the conservative default value, actually
means "it _may_ be overlayed" while `overlayed === false` means
"this is absolutely not overlayed".
I think it should be named as `nonoverlayed`, and then a query name like
`is_nonoverlayed` would be more sensible than the alternative
`is_overlayed`, which actually should be something like `can_be_overlayed`.
This commit adds new reflection utility named `Base.infer_effects` that
works in the same way as `Base.return_types` but returns inferred effects
instead. It would be helpful to test that certain method call has an
expected effects.

For example, we can now remove `Base.@pure` annotation from the definition of
`BroadcastStyle(a::A, b::B) where {A<:AbstractArrayStyle{M},B<:AbstractArrayStyle{N}} where {M,N}`
and checks it's still eligible for concrete evaluation like this
(see <#44776 (comment)> for the context):
```julia
julia> import Base.Broadcast: AbstractArrayStyle, DefaultArrayStyle, Unknown

julia> function BroadcastStyle(a::A, b::B) where {A<:AbstractArrayStyle{M},B<:AbstractArrayStyle{N}} where {M,N}
           if Base.typename(A) === Base.typename(B)
               return A(Val(max(M, N)))
           end
           return Unknown()
       end
BroadcastStyle (generic function with 1 method)

julia> # test that the above definition is eligible for concrete evaluation
       @test Base.infer_effects(BroadcastStyle, (DefaultArrayStyle{1},DefaultArrayStyle{2},)) |> Core.Compiler.is_total_or_error
Test Passed
```

Co-authored-by: Takafumi Arakaki <[email protected]>
…44808)

Although they are basically equivalent in the current implementation,
it would be more correct conceptually if we propagate `TRISTATE_UNKNOWN`
instead of `ALWAYS_TRUE`, as it is hard or impossible to derive a global
conclusion from a local analysis on each statement in most places.

This commit also adds a docstring that simply explains the design of the
effect analysis.
)

This setting is particularly useful since it allows the compiler to
evaluate a call of the applied method when all the call arguments are
fully known, no matter if the call results in an error or not.

This commit also adds some more explanations on the difference between
`@pure` and `@assume_effects`.
Just found this case from code review, but this `Vararg` should be widened.
Now our abstract interpretation system caches two separate lattice
properties return type and call effects, and the previous optimization
to avoid adding backedge for a frame whose return type is `Any` turns
out to be insufficient -- now we also need to make sure that the
inferred effects don't provide any useful IPO information.

Example:
```julia
julia> const CONST_DICT = let d = Dict()
           for c in 'A':'z'
               push!(d, c => Int(c))
           end
           d
       end;

julia> Base.@assume_effects :total_may_throw getcharid(c) = CONST_DICT[c];

julia> @noinline callf(f, args...) = f(args...);

julia> function entry_to_be_invalidated(c)
           return callf(getcharid, c)
       end;

julia> @test Base.infer_effects((Char,)) do x
           entry_to_be_invalidated(x)
       end |> Core.Compiler.is_concrete_eval_eligible
Test Passed

julia> @test fully_eliminated(; retval=97) do
           entry_to_be_invalidated('a')
       end
Test Passed

julia> getcharid(c) = CONST_DICT[c]; # now this is not eligible for concrete evaluation

julia> @test Base.infer_effects((Char,)) do x
           entry_to_be_invalidated(x)
       end |> !Core.Compiler.is_concrete_eval_eligible
Test Failed at REPL[10]:1
  Expression: Base.infer_effects((Char,)) do x
        entry_to_be_invalidated(x)
    end |> !(Core.Compiler.is_concrete_eval_eligible)
ERROR: There was an error during testing

julia> @test !fully_eliminated() do
           entry_to_be_invalidated('a')
       end
Test Failed at REPL[11]:1
  Expression: !(fully_eliminated() do
        entry_to_be_invalidated('a')
    end)
ERROR: There was an error during testing
```
This reduces invalidations when loading packages that define new
AbstractString types.

(cherry picked from commit 515a242)
Acquiring this lock in many implementations could result in deadlock,
even with an existing reader. Add a TLS check for reentry before, instead
of relying on the implementation specifics, to avoid any issues.

(cherry picked from commit 7df454b)
Put back a few lines that were removed by PR $42703 because they are required to successfully embed Julia with MSVC.

(cherry picked from commit 209aad1)
Note: this is the first build of the real upstream version 1.2.12 which was
released a few days ago.

(cherry picked from commit 81e7cfc)
We were accidentally passing the start of the list instead of the end of
the list, resulting in some values passing through uninitialized.

Fix #42297 regression

(cherry picked from commit 5f2abf6)
They look like this:
```
    CC src/processor.o
In file included from /Users/mhorn/Projekte/Julia/julia.master/src/processor.cpp:10:
In file included from ./processor.h:5:
./julia.h:395:9: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
        struct {
        ^
./julia.h:405:9: warning: anonymous types declared in an anonymous union are an extension [-Wnested-anon-types]
        struct {
        ^
2 warnings generated.
```
and come from code that was introduced by @Keno in PR #43852.

But it turns out that the union is not used at all! So I'm simply removing
the offending union. Perhaps it is needed for some future work, but it should
be trivial to add it back if needed. If that happens, I suggest a comment
is added that explain why this looks similar to but has different layout
compared to the `typedef _jl_purity_overrides_t` also in `julia.h`.

(cherry picked from commit bb91e62)
(cherry picked from commit b5bbb9f)
The event might have triggered on another thread before we observed it
here, or it might have gotten finalized before it got triggered. Either
outcome can result in a lost event. (I observed the later situation
occurring locally during the Dates test once).

(cherry picked from commit 8cc00ff)
…#44972)

* [REPL] remove erroneous space in test
  Introduced by #33805 (74f2de1).
* Revert "Temporarily move the `REPL` test suite to node 1, to buy us time until we fix the underlying bugs (#44961)"
  This reverts commit 322fd70.

(cherry picked from commit fbec395)
(cherry picked from commit c589e0d)
If we load libjulia-codegen directly, e.g. from opt or llc,
we don't call julia_init but init_llvm.

Co-authored-by: Keno Fischer <[email protected]>
(cherry picked from commit f562ad0)
giordano and others added 8 commits May 23, 2022 09:42
By default Clang on Linux defines the macro `__GNUC__`, so to guard GCC-specific
code paths it isn't sufficient to check `#ifdef __GNUC__`.

(cherry picked from commit bcc0f70)
* Apply patch for GMP CVE-2021-43618

* Update checksums

(cherry picked from commit dea9805)
Otherwise, overrides do not trigger when using `artifact"..."` inside a
submodule.

(cherry picked from commit 9b106ad)
(cherry picked from commit 3d6731b)
Set default blas num threads to Sys.CPU_THREADS / 2 in absence of OPENBLAS_NUM_THREADS

Co-authored-by: SamuraiAku <[email protected]>
(cherry picked from commit 390503e)
This fixes an issue where an error would be thrown in the REPL if you tried to
transpose an input that was a single character while your cursor was to the
right of that character (e.g., "A|"). To fix this, let's move left once before
we check for if we're at the start of a line. This does change behavior slightly
in that the cursor can move left once without actually transposing anything, but
this seems to match what Emacs does with M-x transpose-chars in an equivalent
situation.

(cherry picked from commit 9dd993e)
@KristofferC
Copy link
Member Author

What is blocking this PR?

There were some dependency updates that fixes security issues that should go in before RC. But should be in pretty good shape now.

@KristofferC
Copy link
Member Author

To check for regressions in this PR itself:

@nanosoldier runtests(ALL, vs = ":release-1.8")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@KristofferC
Copy link
Member Author

@nanosoldier runtests(["AbstractAlgebra", "AdvancedVI", "BayesianQuadrature", "CachePath", "ChainRules", "CropRootBox", "FHIRClient", "GaussianMixtureAlignment", "HighDimPDE", "HydrophoneCalibrations", "KissMCMC", "Knet", "MatrixEquations", "MatrixProfile", "MeshMaker", "PowerSimulationsDynamics", "PyCallJLD", "QuantumTomography", "Relief", "ResizingTools", "SlackThreads", "StoppingInterface", "TensorKit", "TuringGLM"], vs = ":release-1.8")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

fxcoudert and others added 2 commits May 24, 2022 18:26
Co-authored-by: KristofferC <[email protected]>
(cherry picked from commit 86f5501)
Distributed closes and destroys stdin, but some tests attempted to
explicitly use it, leading to test problems. We previously interpreted
this as passing devnull, but this is better to be explicit.

(cherry picked from commit 64a86f9)
@maleadt
Copy link
Member

maleadt commented May 24, 2022

Pushed a manual backport of #45016. Noticed while doing so that #44967 wasn't marked for backport, maybe we need to include it.

@KristofferC
Copy link
Member Author

@nanosoldier runtests(["AbstractAlgebra", "AdvancedVI", "BayesianQuadrature", "CachePath", "ChainRules", "CropRootBox", "FHIRClient", "GaussianMixtureAlignment", "HighDimPDE", "HydrophoneCalibrations", "KissMCMC", "Knet", "MatrixEquations", "MatrixProfile", "MeshMaker", "PowerSimulationsDynamics", "PyCallJLD", "QuantumTomography", "Relief", "ResizingTools", "SlackThreads", "StoppingInterface", "TensorKit", "TuringGLM"], vs = ":release-1.8")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@KristofferC KristofferC merged commit f9f6789 into release-1.8 May 26, 2022
@KristofferC KristofferC deleted the backports-release-1.8 branch May 26, 2022 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release management and versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.